Remove R development dependencies #4306
Conversation
environment.yml
Outdated
| - r-git2r # dependency of lintr | ||
| - r-lintr ==3.1.2 | ||
| - r-styler ==1.10.3 | ||
| - r-lintr | ||
| - r-styler |
There was a problem hiding this comment.
We should be able to get rid of those if we re-enable the pre-commit hook at
ESMValTool/.pre-commit-config.yaml
Lines 45 to 53 in 9bde7d3
and perform the currently failing test using pre-commit instead.
We would just need to pin that pre-commit hook to the version where the checks passed (i.e. the version of the pre-commit hook that uses the versions of lintr and styler listed here).
There was a problem hiding this comment.
all fun and games until I realized I lost you at pinning the pre-commit hook - how the heck would we know the version when that hook last passed the test? Unless you want me to write a Monte Carlo routine that loops over lintr hook versions 🤣
There was a problem hiding this comment.
can we not just skip the R-linting test in tests/unit/test_lint.py? I've not seen any of the following in the past 5 years:
- anyone writing a recipe in R
- anyone complaining that our R recipes don't follow R linting standards
There was a problem hiding this comment.
actually, scratch that - it's prob a good idea to have an r-lintr test, let me see what I can do with that pre-commit version, I'll toss some coins 🪙
There was a problem hiding this comment.
I've not forgot about this, am gonna try have a look at it tomorrow - sorry, I've been under a lot of snow elsewhere, bud
There was a problem hiding this comment.
pre-commit config file brought back to original state in a2c5d63 - off to the docs to add the mention about style etc
There was a problem hiding this comment.
docs modded in 88f3e41 - do we want to tell R folks to use the pre-commit way or r-linrs/styler way?
There was a problem hiding this comment.
Thanks for the changes V! I pushed a few additions to the docs in fdddae6 and removed the unit test that checks compliance with lintr as we want this done with pre-commit instead.
There was a problem hiding this comment.
Looks great! I had doubts about keeping that fully skipped test, but I was afraid we'd have to use it again in the future should the R hook be left undeveloped (it diesn't look too actively maintained). Shall we merge then? 🍺
There was a problem hiding this comment.
Yes, but let's first fix the docs build
|
Anything I can do to help getting this merged? I can't install ESMValTool on Levante anymore because I always run into errors after 8+ hours of solving environments 😄 |
it's just about there, Manu, remove the r-lintr and r-styler from your env on Levante, and observe the difference 😁 |
|
docs failing is endemic, I'll look into it right now https://app.readthedocs.org/projects/esmvaltool/builds/31083499/ |
Update docs Re-enable pre-commit in GHA
le greene le @bouweandela 🟢 |
bouweandela
changed the title
|
very many thanks @bouweandela 🍻 |
3 participants
Description
Our Python 3.13 + friends environment is not solving anymore https://github.com/ESMValGroup/ESMValTool/actions/runs/20801973056/job/59748587368 but given the OSX (pure Python one) solves well, it must be something to do with friends (R, NCL, etc)
This pull request removes the R development dependencies and refers R contributors to pre-commit.
Documentation: https://esmvaltool--4306.org.readthedocs.build/en/4306/community/code_documentation.html#r
Before you get started
Checklist
It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.